Glasgow | 25-ITP-SEP | Shreef Ibrahim | Sprint 2| Coursework #793
Conversation
…eguency of each items in tally.js file
shreefAhmedM
added
the
Needs Review
| // console.log(value); | ||
| // } | ||
| // It will log syntax Error because Objects themselves aren’t directly iterable Unlike arrays we need to converted | ||
| // and then we can loop |
There was a problem hiding this comment.
Very good explanation — you clearly identified why the original loop didn't work and how to resolve it. @shreefAhmedM
| // it will log [object Object]. | ||
| console.log(`${recipe.title} serves ${recipe.serves}`); | ||
| console.log("ingredients"); | ||
| recipe.ingredients.forEach((ingredient) => { |
There was a problem hiding this comment.
Using forEach is a clean and readable way to loop through the ingredients. Good choice.
| // When passed to contains with a non-existent property name | ||
| // Then it should return false | ||
|
|
||
| test("Given an object with none-existing property should return false", () => { |
| function contains() {} | ||
|
|
||
| function contains(obj, prop) { | ||
| return obj && typeof obj === "object" && prop in obj; |
There was a problem hiding this comment.
I like that you safeguard against non-object inputs by checking typeof obj === "object". It makes the function more robust and prevents errors when unexpected values are passed in.
| // is trying to access a property using address[0] and address is an object, not an array. | ||
| // it will log undefined. | ||
| // to access the houseNumber property, we should use dot notation or bracket notation with string,and either | ||
| // way using object.key or object["key"] |
There was a problem hiding this comment.
Nice work @shreefAhmedM!
I think we can simplify the comment a bit.
| // Given invalid parameters like an array | ||
| // When passed to contains | ||
| // Then it should return false or throw an error | ||
| test("Given an object with an existing property should return true", () => { |
There was a problem hiding this comment.
@shreefAhmedM Good test — small mismatch: the acceptance criteria describe invalid parameters (like an array), but the test title says it's testing an object with an existing property.
| @@ -1,3 +1,5 @@ | |||
| function contains() {} | |||
|
|
|||
| function contains(obj, prop) { | |||
There was a problem hiding this comment.
@shreefAhmedM we can make the argument prop more descriptive
| @@ -1,5 +1,15 @@ | |||
| function createLookup() { | |||
| // implementation here | |||
| function createLookup(nestedArr) { | |||
There was a problem hiding this comment.
Good job — this works and produces the correct result! 👍
One suggestion: flattening the array isn’t necessary here, since the input is already a 2D array of [country, currency] pairs.
We can make the implementation simpler by iterating directly over each pair and assigning the key-value in the object. This keeps the code readable and avoids potential issues with unexpected nested arrays.
| return result; | ||
| } | ||
|
|
||
| createLookup([ |
There was a problem hiding this comment.
We don’t need to call createLookup here in the module. The tests will call it with the required inputs
| // Given an array with duplicate items | ||
| // When passed to tally | ||
| // Then it should return counts for each unique item | ||
| test("Given a function tally(['a']), target output should be { a: 1 }", () => { |
There was a problem hiding this comment.
We can make the test name align with the test requirement
| // Given an empty array | ||
| // When passed to tally | ||
| // Then it should return an empty object | ||
| test.todo("tally on an empty array returns an empty object"); |
There was a problem hiding this comment.
The test.todo is only a placeholder. Once you implement the real test for an empty array, you can remove this line.
| // When passed an array of items | ||
| // Then it should return an object containing the count for each unique item | ||
|
|
||
| test("Given a function tally(['a']), target output should be { a: 1 }", () => { |
There was a problem hiding this comment.
The test title and implementation don’t align.
| function countWords(str) { | ||
| const obj = {}; | ||
| let tidyStr = str.replace(/[!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~]/g, ""); | ||
| let arr = tidyStr.toLowerCase().split(" "); |
There was a problem hiding this comment.
Just thinking — splitting by " " only handles regular spaces. It won’t handle tabs (\t) or multiple consecutive spaces correctly. Can we think of a way to handle this?
|
|
||
| // Find the value with the highest frequency | ||
| // Find the value with the highest frequency | ||
| function highestFreg(freqs) { |
There was a problem hiding this comment.
@shreefAhmedM Consider renaming highestFreg to highestFreq to fix the typo.
jaymes15
added
Review in progress
2 participants
Self checklist
Her is JavaScript challenges tasks covering running Jest test, error fixing, code interpretation, and a stretch exploration activity.